Use vm.Script and refactor coverage caching.
#1239
Conversation
ghost
added
the
cpojer
force-pushed
the
master
branch
2 times, most recently
from
5bf0bf5 to
347990d
Compare
|
To further improve performance I tried to use However, it yields no performance benefits that I can identify. I ran Jest twice, once writing the bytecode and once with reading from cache. I then compared this PR with the code in the gist above and ran Jest multiple times. I could not identify a statistically relevant performance difference when using this feature, unfortunately. I thought that parsing and compiling a script would take up a relevant portion of the runtime but it doesn't seem relevant. It seems like the bytecode cache is about 2-3 times smaller than the source text, which would be a nice win for memory and cache size. However, a See nodejs/node@96934cb and https://nodejs.org/api/vm.html#vm_class_vm_script for the node.js feature. The rest of this PR is still relevant, of course. |
|
@cpojer does it set |
|
cachedDataProduced is false, yes, so everything appears to be working correctly. I only tested on a small size of files, however, which might have something to do with this. What do you think about the chances of adding |
|
I just tested it on a larger number of files (453) and the difference seems to be less than 10ms. Is this expected? Is parsing and compiling so fast these days that everything else just dwarfs it? |
|
@cpojer what about I don't think that there is a huge difference when re-using cached data for small files. |
|
Adding Are you saying this feature is not very useful for many small files but would be more useful for a few big files? Is there anything we can tweak to make this more useful? |
|
@cpojer ok, good to know! Thank you! Honestly saying, I don't know it for sure. From my local test the |
| const instrumentor = new istanbul.Instrumenter(); | ||
| const source = instrumentor.instrumentSync(sourceText, filename); | ||
| const tracker = instrumentor.currentState.trackerVar; | ||
| return source + storageVarName + '.coverState=' + tracker + ';'; |
There was a problem hiding this comment.
what does this line do?
There was a problem hiding this comment.
Basically right now we create one instrumenter per test file, so this just makes sure that we put the coverage info into the $JEST variable so we can pull it out later.
I'm sure you'll rewrite and fix this soon, however :)
cpojer
force-pushed
the
master
branch
7 times, most recently
from
08f2637 to
322b078
Compare
This ensures we are using the latest version of every package.
* Added a transform.js test. * Relax jest.mock babel transform restrictions. * Updated package.json and .npmignore for all packages.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This diff:
jest --coverage. We now no longer transform files more than once when they are unchanged and coverage is used.